test(search): pin a route-level Supabase round-trip budget for /api/search - #1950
Conversation
Ledger #98 residual: the offline round-trip budget harness pinned the answer path and the search retrieval core (searchChunksWithTelemetry) but left the /api/search route itself invisible — auth, rate limiting, scope resolution, related-document enrichment, and the telemetry write were untested. Drive POST directly with a counted Supabase client, following the tests/answer-route-preamble.test.ts pattern of importing the route fresh per scenario with vi.doMock, so a round trip added anywhere in the route preamble or post-processing becomes a red gate. This establishes the harness and non-vacuity checks only; pinning the exact count/shape is ledger #189's follow-up commit. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QLbw9qpfjv5CeNz6XpmteN
…he ×3 text-RPC fan-out Ledger #189: tighten tests/search-route-round-trip-budget.test.ts from "the harness exists and observes traffic" to an explicit pinned count (16) and exact breakdown, so a route round trip added or removed is a red gate rather than something a reviewer has to notice. Also traced why match_document_chunks_text_v2 and match_document_table_facts_text_v2 are each issued 3 times per search (already pinned separately at the retrieval-core layer by tests/search-round-trip-budget.test.ts): rag-candidate-sources.ts fans out up to maxTextRpcQueryVariants (3) lexical query-variant phrasings — primary plus up to two siblings — per text surface, to rescue recall when the primary phrasing misses. A PT-02 early exit (firstVariantPoolIsStrong in rag-retrieval-variants.ts) already skips the sibling RPCs when the primary pool is deep and precisely anchored, so this ×3 is the fixture's worst-case shape (a single mocked chunk never clears the early-exit depth bar), not the typical cost against a well-matched corpus. Disposition: intentional, keep as-is — no code change in this PR. Full reasoning in the PR body per ledger #189's scope (decide-and-record, not collapse-without-canary). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QLbw9qpfjv5CeNz6XpmteN
npm run typecheck caught TS2416: RouteQuery.then() reused a single type parameter for both the fulfilled and rejected branches, which is narrower than lib.es5's PromiseLike<T>.then<TResult1, TResult2>(...). Match the base interface's two-type-parameter signature so `.insert().then(undefined, onRejected)` (the pattern logWeakSearch uses) type-checks. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QLbw9qpfjv5CeNz6XpmteN
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 43 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
Comment |
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QLbw9qpfjv5CeNz6XpmteN
…merged #215, #210, and #293 were queued as `done` in this same PR, each citing a PR (#1949, #1953, #1962) that turned out still to be open, not merged — verified by checking whether each PR's actual file change is present on origin/main (none are). Reconciling the original `done` requests would have closed these rows while their fixes exist only on unmerged branches, one of which (#1949) currently has failing required CI. Converts all three to `update` requests documenting the correction and the real current state, so reconciling this PR now cannot close a row before its fix has actually landed. Leaves #98, #189, and #194 as `done` unchanged — those cite PR #1950 and PR #1947, both confirmed merged into main.
Summary
searchChunksWithTelemetry, intests/search-round-trip-budget.test.ts), but not the/api/searchroute itself — its auth, rate limiting, scope resolution, related-document enrichment, and telemetry write were invisible to any budget test. Addtests/search-route-round-trip-budget.test.ts, drivingPOSTfromsrc/app/api/search/route.tswith a counted Supabase client, following thetests/answer-route-preamble.test.tspattern of importing the route fresh per scenario withvi.doMock. A round trip added anywhere in the route preamble or post-processing is now a red gate instead of something a reviewer has to notice.match_document_chunks_text_v2andmatch_document_table_facts_text_v2are each issued up to 3 times per search —src/lib/rag/rag-candidate-sources.tsfans outmaxTextRpcQueryVariants(3) lexical query-variant phrasings (primary + up to two siblings) per text surface, to rescue recall when the primary phrasing misses. This already has a cost-reducing early exit:firstVariantPoolIsStrong(src/lib/rag/rag-retrieval-variants.ts, the "PT-02" mechanism) skips the sibling RPCs once the primary variant's pool is deep and precisely anchored, so a real, well-matched corpus query pays less than ×3 in practice — this test's single-chunk fixture never clears that depth bar, so it measures the worst-case fan-out, not the typical cost. Collapsing the fan-out further (e.g. to a single variant) would be a retrieval-behaviour change and needs the RAG-flag + live-canary process this PR is explicitly out of scope for; no such change is made here.Three commits, each independently revertible: (1) the route-level harness + non-vacuity checks, (2) pinning the exact count/shape + the ×3 disposition, (3) a small
PromiseLike<T>generics fix caught bynpm run typecheck(the test harness's mocked query builder needed a two-type-parameterthen()to satisfylib.es5'sPromiseLike<T>when a caller passes bothonfulfilledandonrejected).Verification
npm run test:focused -- --files tests/search-route-round-trip-budget.test.ts— declined, exactly as designed ("Focused test selection is unsafe: test or configuration paths changed"); it fails closed for test-file changes and prints its own follow-up instruction. Ran the full suite instead, per that instruction.npm run verify:pr-local -- --files tests/search-route-round-trip-budget.test.ts— full run (this diff is executable/test scope, so it escalates past the docs/workflow fast path). Decisive output:check:runtime— PASS (Node 24.19.0, npm 11.17.0)check:installed-lock-parity— PASSformat:changed— "All matched files use Prettier code style!"lint— clean (0 warnings/errors acrosssrc tests scripts worker supabase playwright+ configs)typecheck— clean (after the fix above)test—Test Files 602 passed (602)/Tests 6511 passed | 4 skipped (6515)check:rag:fixtures— passedcheck:medication-interactions— "data/medication-interaction-index.json is up to date (523 rows)."check:medication-lexicon-reportreportsdocs/medication-interaction-lexicon-review.mdas stale. This diff touches exactly one file (tests/search-route-round-trip-budget.test.ts— confirmed viagit diff origin/main --name-only); the lexicon doc and its generator were last touched by fix(medications): reach lithium, retire the dead z-drugs term, make lexicon review flags evidence-bearing #1923, well before this branch's point (7a157f4). This is a pre-existing staleness onmain, not something this PR introduced.npm run verify:ui— not applicable, no UI/routing/styling/browser change.npm run verify:release— not run; not a release/handoff-confidence request.npm run eval:retrieval:quality— not applicable; no retrieval, ranking, selection, chunking, or scoring behaviour change (this PR only observes existing behaviour through mocked round-trip counting; it changes no production code undersrc/lib/rag/**or the route itself).npm run eval:rag/npm run eval:quality— not applicable; no answer-generation or synthesis-prompt change.npm run check:production-readiness— not applicable; no clinical workflow, privacy, environment, Supabase, or source-governance behaviour change (test-only diff).Risk and rollout
RAG_PROVIDER_MODE=offline, a stubbed@/lib/supabase/adminclient, and stubbed@/lib/openai. No network, database, or provider calls.Notes
tests/search-route-round-trip-budget.test.ts— it does not modifysrc/app/api/search/route.ts,src/lib/rag/**, or any other production/ranking surface, so no Clinical Governance Preflight orRAG impact:line applies (confirmed againstscripts/pr-policy.mjs'sclassifyPullRequestFiles: the changed path matches neitherclinicalRiskPatternsnorragRankingPatterns).Generated by Claude Code